- 
                Notifications
    You must be signed in to change notification settings 
- Fork 142
Flexible playground components #298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
6e66134    to
    ecbc38c      
    Compare
  
    1f1f7ba    to
    869956d      
    Compare
  
    ecbc38c    to
    a385a1a      
    Compare
  
    869956d    to
    84ac01e      
    Compare
  
    
      
        
              This comment was marked as resolved.
        
        
      
    
  This comment was marked as resolved.
84ac01e    to
    4310736      
    Compare
  
    | Ready for review now. | 
4310736    to
    49d6383      
    Compare
  
    49d6383    to
    5c712c1      
    Compare
  
    | @Skaiir @barmac @philippfromme, maybe any chance to get a review for this? 🙂 | 
| I missed this. Thanks for notice. | 
| { | ||
| name: 'data', | ||
| attachFn: 'attachDataContainer', | ||
| selector: 'cm-editor' | ||
| }, | ||
| { | ||
| name: 'result', | ||
| attachFn: 'attachResultContainer', | ||
| selector: 'cm-editor' | ||
| }, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these two have the same selector?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about adding a custom class name to each Code Mirror instance. However, it was not possible (at least not clearly documented). That's why I keep things as they are; you can still handle/style each editor via parent selectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was afraid that having the same selector in the tests can make us think data container is attached while it's the result container. If that's not the case, we can ignore it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we now remove every container after each test, it shouldn't be the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure the test cases don't cause us pain in the future, and also enable TS users to consume the new API :)
5c712c1    to
    69fba93      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge it!
Closes #292
This makes it possible to attach all playground components individually. This enables a mode for the playground to render nothing (by not providing
containerconfig) but only orchestrate the different instances. This is a requirement to create a product-specific playground version (cf. https://github.com/bpmn-io/internal-docs/issues/527).This also makes the "actions" (download, embed) optional.
Overview components
Demo
Check out this demo to get a feeling of what you can do with the API changes.